InitContext, part 4 - Use init!! to replace evaluate_and_sample!!, predict, returned, and initialize_values
#984
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 tasks
Benchmark Report for Commit 5025592Computer InformationBenchmark Results |
penelopeysm
commented
Jul 10, 2025
ext/DynamicPPLMCMCChainsExt.jl
Outdated
Comment on lines
126
to
134
| # Extract values from the chain | ||
| values_dict = chain_sample_to_varname_dict(parameter_only_chain, sample_idx, chain_idx) | ||
| # Resample any variables that are not present in `values_dict` | ||
| _, varinfo = last( | ||
| DynamicPPL.init!!( | ||
| rng, | ||
| model, | ||
| varinfo, | ||
| DynamicPPL.ParamsInit(values_dict, DynamicPPL.PriorInit()), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Note that, if the chain does not store varnames inside its info field, chain_sample_to_varname_dict will fail.
I don't consider this to be a problem right now because every chain obtained via Turing's sample() will contain varnames:
So this is only a problem if you manually construct a chain and try to call predict on it, which I think is a highly unlikely workflow (and I'm happy to wait for people to complain if it fails). There are a few places where this happened in the test suite. I fixed them all by adding the appropriate varname dictionary.
However, it's obviously ugly. The only good way around this is, as I suggested before, to rework MCMCChains.jl.
penelopeysm
changed the title
init!! to replace evaluate_and_sample!!, predict, returned, and initialize_valuesInitContext, part 4 - Use init!! to replace evaluate_and_sample!!, predict, returned, and initialize_values
penelopeysm
force-pushed
the
py/init-prior-uniform
branch
2 times, most recently
from
025aa8b to
b55c1e1
Compare
penelopeysm
force-pushed
the
py/actually-use-init
branch
5 times, most recently
from
b72c3bf to
92d3542
Compare
penelopeysm
force-pushed
the
py/actually-use-init
branch
4 times, most recently
from
7438b23 to
d55d378
Compare
penelopeysm
force-pushed
the
py/actually-use-init
branch
3 times, most recently
from
12d93e5 to
7a8e7e3
Compare
penelopeysm
force-pushed
the
py/actually-use-init
branch
from
7e38bbe to
1d8bceb
Compare
penelopeysm
force-pushed
the
py/actually-use-init
branch
from
1d8bceb to
2edcd10
Compare
|
DynamicPPL.jl documentation for PR #984 is available at: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## py/init-prior-uniform #984 +/- ##
=========================================================
- Coverage 82.20% 80.44% -1.77%
=========================================================
Files 39 39
Lines 4052 3998 -54
=========================================================
- Hits 3331 3216 -115
- Misses 721 782 +61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part 1: Adding
hasvalueandgetvalueto AbstractPPLPart 2: Removing
hasvalueandgetvaluefrom DynamicPPLPart 3: Introducing
InitContextandinit!!This is part 4/N of #967.
In Part 3 we introduced
InitContext. This PR makes use of the functionality in there to replace a bunch of code that no longer needs to exist:setval_and_resample!followed by model evaluation: This process was used forpredictandreturned, to manually store certain values in the VarInfo, which would be used in the subsequent model evaluation. We can now do this in a single step usingParamsInit.initialize_values!!: very similar to the above. It would manually set values inside the varinfo, and then it would trigger an extra model evaluation to update the logp field. Again, this is directly replaced withParamsInit.evaluate_and_sample!!: direct one-to-one replacement withinit!!.There is one API change associated with this: the
initial_paramskwarg tosamplemust now be anAbstractInitStrategy. It's still optional (it will usually default toPriorInit). However, there are two implications:initial_paramscannot be a vector of parameters anymore. It must beParamsInit(::NamedTuple)ORParamsInit(::AbstractDict{VarName}).ParamsInitexpects values in unlinked space,initial_paramsmust always be specified in unlinked space. Previously,initial_paramswould have to be specified in a way that matched the linking status of the underlying varinfo.I consider both of these to be a major win for clarity. (One might argue that vectors are more convenient. Sure, you can get a vector with
vi[:], but now you can just dovalues_as(vi, Dict{VarName,Any})instead.)Closes
Closes #774
Closes #797
DOES NOT CLOSE #983
Closes TuringLang/Turing.jl#2476
Closes TuringLang/Turing.jl#1775